Skip to content

feat(catalog): add OAuth server setup flow - #113

Open
vishu-bh wants to merge 4 commits into
mainfrom
feature/catalog-oauth-add-flow
Open

feat(catalog): add OAuth server setup flow#113
vishu-bh wants to merge 4 commits into
mainfrom
feature/catalog-oauth-add-flow

Conversation

@vishu-bh

@vishu-bh vishu-bh commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add an OAuth-specific catalog setup dialog built from existing shadcn primitives.
  • Register credentials, launch authorization, enable the server, then discover tools.
  • Surface caller-scoped OAuth status and re-authorization actions on catalog cards.

Backend dependency

Requires backend PRs IBM/mcp-context-forge#6588 and IBM/mcp-context-forge#6620. Handwritten frontend contract types are temporary until OpenAPI generation includes the merged API.

Validation

  • npm run lint -- --max-warnings=0
  • npx tsc --noEmit
  • npm run test -- --run src/pages/ServerCatalog.test.tsx src/components/server-catalog/CatalogResults.test.tsx src/api/catalog.test.ts

NOTE

Please do not merge this till backend dependency is resolved. For the purpose of development handwritten contracts is added but it needs to be cleaned up once backend dependency is resolved

Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
vishu-bh marked this pull request as ready for review September 9, 2026 11:48
@vishu-bh
vishu-bh force-pushed the feature/catalog-oauth-add-flow branch from 4390417 to 700cf15 Compare September 9, 2026 11:51
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
vishu-bh requested review from gcgoncalves and marekdano and removed request for marekdano September 9, 2026 12:55
@marekdano
marekdano removed their request for review September 9, 2026 13:27

@marekdano marekdano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1. 🔴 High — OAuth popup blocked on first-time setup

File: src/pages/ServerCatalog.tsx:638

handleOAuthSubmit does await registerServer(...) (an HTTP POST) before calling serversApi.triggerOAuthAuthorization(gatewayId). By the time window.open() runs inside that call, the browser no longer treats it as a direct response to the click, so popup blockers will block it (authWindow is null) — this will trigger on essentially every first-time OAuth registration.

Failure scenario: User clicks "Configure and authorize" on a new OAuth catalog server → popup blocked → "Failed to open OAuth authorization window" error. Retrying then hits the pendingOAuthGatewayId fast path, which calls triggerOAuthAuthorization synchronously and succeeds — so it looks like a transient glitch but is actually deterministic.


2. 🟠 Medium-High — Removed auth-type filter exposes unsupported servers

File: src/pages/ServerCatalog.tsx:454

The catalog's auth-type allow-list filter (previously SUPPORTED_AUTH_TYPE_SET) was removed. supportedServers = data?.servers ?? [] no longer filters by auth type, so servers with types outside Open/API Key/OAuth (e.g. Basic, mTLS) now render and are "Add"-able.

Failure scenario: Clicking "Add" on such a server falls through handleAdd's if/else chain straight to void registerServer(server) with no credentials — likely a generic server-side failure, or a silent no-auth registration. No test covers this path.


3. 🟡 Medium — Disconnect/Test not blocked during in-flight authorization

File: src/components/server-catalog/CatalogResults.tsx:202

The new Authorize action shares the isAdding pending flag with Test and Disconnect, but Test's/Disconnect's own disabled conditions were never updated to also check isAdding.

Failure scenario: User clicks Authorize (popup opens, isAdding true) → reopens the dropdown and clicks Disconnect (only checks isTesting || isDisconnecting) → gateway is deleted mid-authorization → when the popup resolves, toggleEnabled/fetchToolsAfterOAuth run against a deleted gateway id, surfacing a confusing error instead of being prevented up front.

Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
vishu-bh requested a review from marekdano September 10, 2026 11:28
@gcgoncalves

Copy link
Copy Markdown
Contributor

NIT: Add aria-required to required form inputs and aria-describedby linking inputs with errors to paragraphs describing them.

Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@a-effort

a-effort commented Sep 11, 2026

Copy link
Copy Markdown
Contributor
  1. handleOAuthSubmit in src/pages/ServerCatalog.tsx opens a popup window before calling registerServer, so popup blockers do not interfere. If registerServer throws, the catch branch returns without closing that window. triggerOAuthAuthorization was never called so nothing closes it. Fix: call authWindow.close() in the catch block.

  2. getOAuthCardState in src/components/server-catalog/CatalogResults.tsx is called for every registered card, not only OAuth ones. For open and api_key servers it falls through to the success default, which is currently correct, but only because no other branch matches. The function checks server.requires_oauth_config and oauthStatuses without first checking server.auth_type, so a future edit to the fallback or a gateway_id collision could silently produce the wrong badge. Suggest gating the whole OAuth state path on server.auth_type === 'OAuth2.1'.

  3. In CatalogOAuthDialog.tsx, the clientSecret validation guard uses clientSecret.trim() to check non-emptiness, but the submitted body sends the raw clientSecret without trimming. A single-space value passes validation and is sent as-is. Every other field is trimmed before submission. If keeping whitespace in the secret is intentional, the guard should use clientSecret.length > 0 instead, and a comment should say so.

  4. CatalogServerRegisterBody types name as string | undefined. The dialog submits name: name.trim() || null, which puts an explicit null on the wire. The backend may treat null and a missing field differently. The API-key dialog omits the field rather than sending null; this one should do the same.

Could you also share a screen recording or screenshots of the OAuth setup flow? Useful to see: the dialog form with validation errors, the card status badges, and the authorize action triggering the popup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants